Support default values for env var expansion #10907
Conversation
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
|
I'd like to make sure we're in alignment with the configuration sig on the syntax before merging this |
|
@TylerHelmuth I though that was settled, per #5228 (comment)
|
|
@mx-psi is Configuration WG decision on shell syntax documented / implemented somewhere? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10907 +/- ##
==========================================
+ Coverage 91.89% 91.90% +0.01%
==========================================
Files 411 411
Lines 19311 19320 +9
==========================================
+ Hits 17745 17756 +11
+ Misses 1217 1216 -1
+ Partials 349 348 -1 ☔ View full report in Codecov by Sentry. |
|
Signed-off-by: Yuri Shkuro <[email protected]>
|
Note that the |
| // `env:NAME_OF_ENVIRONMENT_VARIABLE:-default_value` | ||
| // | ||
| // An error message for unset variable can be provided after :? suffix, for example: | ||
| // `env:NAME_OF_ENVIRONMENT_VARIABLE:?error_message` |
There was a problem hiding this comment.
I'm not aware of a definition for setting an error message, is there an open issue somewhere?
There was a problem hiding this comment.
I think we should still have it. I don't understand the approach "let's be like Shell but just different enough so that common things don't work".
There was a problem hiding this comment.
not disagreeing, this was me asking if there is already a discussion about this somewhere I haven't found yet
There was a problem hiding this comment.
I don't understand the approach "let's be like Shell but just different enough so that common things don't work".
This doesn't mean that we need to add full support for Bash/POSIX-like syntax today. Your PR does not add full support for all the features that Bash supports and that's okay. As I said above, I am not opposed to adding this, but since alignment with the Configuration WG is an explicit goal we should bring it up on their repo first.
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
|
I removed error syntax -- 3a3f939. |
Signed-off-by: Yuri Shkuro <[email protected]>
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
…ry#10907) #### Description Support shell-style default value and error message for env var expansion. ``` // A default value for unset variable can be provided after :- suffix, for example: // `env:NAME_OF_ENVIRONMENT_VARIABLE:-default_value` ``` #### Link to tracking issue Fixes open-telemetry#5228 #### Testing Unit tests #### Documentation * [x] Provider Go docs * [ ] Probably needs some other docs changes? --------- Signed-off-by: Yuri Shkuro <[email protected]>
yurishkuro
changed the title
Description
Support shell-style default value and error message for env var expansion.
Link to tracking issue
Fixes #5228
Testing
Unit tests
Documentation